samples/guestbook: add sample application #68

zombiezen · 2018-06-04T20:06:44Z

This is a cleaned up sample code version of my Guestbook demo from a few weeks ago. Please review and let me know what you think.

This is too large for a "Hello, World!" snippet, since it is targeted at exercising all parts of the SDK (all clouds and all generic interfaces in one binary). What you can glean from this demo is a sense for what a mid-sized "go-cloud" application configured with Wire would look like.

Thanks, linter!

jeanbza · 2018-06-05T02:40:35Z

samples/guestbook/main.go

@@ -261,7 +261,9 @@ func (app *app) serveBlob(w http.ResponseWriter, r *http.Request) {
 		w.Header().Set("Content-Type", "application/octet-stream")
 	}
 	w.Header().Set("Content-Length", strconv.FormatInt(blobRead.Size(), 10))
-	io.Copy(w, blobRead)
+	if _, err = io.Copy(w, blobRead); err != nil {
+		log.Println("Copying blob:", err)


log.Fatal perhaps?

No, because this would mean that a client could kill the whole server.

jeanbza · 2018-06-05T02:46:49Z

samples/guestbook/README.md

+$ gsutil iam ch \
+    serviceAccount:${GCP_IAM_ACCOUNT}:objectViewer \
+    gs://$GCS_BUCKET_NAME
+# On your own: download a GCP icon to gcp.png.


Might be a good idea to put this further up. A lot of users would probably want to copy paste all of this in one go; it'd be a very nice experience if the instructions are set up to allow them to do that.

jeanbza · 2018-06-05T02:47:57Z

samples/guestbook/README.md

+select.
+
+```shell
+$ PROJECT_ID=...


A lot of users would probably want to copy paste all of this in one go; it'd be nice if we didn't have dollar signs. While they convey "this is a shell prompt", it seems to me a small gain to lose copy paste-ability.

Not sure if some syntax guide talks about this, though, or if we have a convention.

Sometimes I've seen people confused that they need to copy-paste into a shell unless they see prompts. I've removed for now.

jeanbza · 2018-06-05T02:48:33Z

samples/guestbook/README.md

+select.
+
+```shell
+$ AWS_ACCT_ID="$( aws sts get-caller-identity --query=Account --output=text )"


Does this assume some logged in state? If so, can we be transparent about that intention further up?

jeanbza · 2018-06-05T02:49:51Z

samples/guestbook/README.md

+$ gcloud iam service-accounts delete $GCP_IAM_ACCOUNT
+```
+
+## Running on Amazon Web Services (AWS)


General comment: these two blocks (running on aws, running on gcp) are very dense. Could we add some comments about what some of the blocks are doing?

Obsoleted by Terraform rewrite.

jeanbza · 2018-06-05T02:50:31Z

samples/guestbook/README.md

+admin@ec2$ AWS_REGION=us-west-1 ./guestbook -env=aws
+```
+
+### Introspection


Could you provide a short sentence describing what this section is about?

+1 to that. A little hand-holding for these samples is invaluable. Nothing too wordy, but a few sentences would go a long way.

This isn't really needed anymore, since the values are now surfaced more succinctly in terraform output.

jeanbza · 2018-06-05T02:51:06Z

samples/guestbook/inject_aws.go

+)
+
+// Configure these to your AWS project.
+const (


Should these be env vars? I think most people in cloud environments (e.g. docker images) are used to getting config into source code via env vars.

Definitely. Don't want to come across as sanctioning hard-coding config and secrets.

Using command-line flags now.

jeanbza · 2018-06-05T02:53:24Z

samples/guestbook/main.go

+	}
+}
+
+// index serves the server's landing page.


Super, super nit: could these comments be terminated equally apart (and, bring line 2 up to line 1)? Line 1 is terminated far sooner than line 2.

This is an ultra nit, feel free to ignore if it doesn't bother you.

Not sure how this happened. 😄 Thanks!

jba · 2018-06-05T11:54:16Z

samples/guestbook/main.go

+		return
+	}
+	defer q.Close()
+	for q.Next() {


To avoid dup of error-handling code, consider:

var err error for q.Next() { ... err = q.Scan(...) if err != nil {break} ... } if err == nil { err = q.Err() } if err != nil { log... }

I read your suggestion slightly incorrectly and filed golang/go#25787. There's a bit too much mutable state for me in your suggestion, so I'm going to keep it as is.

enocom · 2018-06-05T15:07:37Z

samples/guestbook/README.md

@@ -0,0 +1,266 @@
+# Guestbook Sample
+


It would be nice to have a few sentences summarizing what this sample does, e.g., "Guestbook is a sample application which records visitors' names. It also demonstrates a common (standard, conventional?) use of go-wire and go-cloud."

enocom · 2018-06-05T15:09:33Z

samples/guestbook/README.md

+```
+
+## Running Locally
+


Again a quick summary here would be nice. "You will need to run a MySQL database using Docker and then migrate that database. Then, you can start the app locally" or something similar.

enocom · 2018-06-05T15:11:44Z

samples/guestbook/README.md

+# Guestbook Sample
+
+## Building
+


Same as above. "You will need to install vgo with ... The vgo vendor step pulls down all required dependencies specified in go.mod"

enocom · 2018-06-05T15:13:22Z

samples/guestbook/README.md

+select.
+
+```shell
+$ PROJECT_ID=...


This is a huge amount of commands, which makes for an error prone UX. Could this be moved into a script with the user just setting a handful of environment variables before executing the script?

Discussed over IM. I didn't know that Terraform was exactly the thing I needed, so I've rewritten this with Terraform. Thanks!

enocom · 2018-06-05T15:14:36Z

samples/guestbook/README.md

+You will also need to modify `inject_aws.go` with the configuration values you
+select.
+
+```shell


Same as above. This should be moved into a script. The average user will be scared away otherwise.

As a lazy software engineer, I would probably give up on this tutorial on seeing these commands honestly. This step should be made as easy as possible, e.g., export these environment variables and run the script.

Also, although it might be obvious to someone who's been deep in the guts of GCP and AWS CLIs, the average user won't understand a lot of this stuff. So, once you move the commands into a script, add copious annotations explaining each step for the curious reader who wants to understand what you've asked them to do to their IaaS account.

enocom · 2018-06-05T15:20:13Z

samples/guestbook/inject_aws.go

+
+//+build wireinject
+
+package main


A package doc would be good here.

Added in main.go.

enocom · 2018-06-05T15:21:12Z

samples/guestbook/inject_aws.go

+	paramstoreVar = "/guestbook/motd"
+)
+
+func setupAWS(ctx context.Context) (*app, func(), error) {


Some brief documentation would be good here. Your users will learn by repetition, so don't be afraid of being explicit.

enocom

Mostly documentation and UX concerns here.

enocom · 2018-06-05T15:21:54Z

samples/guestbook/inject_aws.go

+	))
+}
+
+func awsBucket(ctx context.Context, cp awsclient.ConfigProvider) (*blob.Bucket, error) {


Documentation here. Again, it's obvious to the long-time user, but when promoting a DI framework, lots of clear explanations will go a long way.

enocom · 2018-06-05T15:22:40Z

samples/guestbook/inject_gcp.go

+	"github.com/google/go-cloud/wire"
+)
+
+// Configure these to your GCP project.


See above about not hard coding these.

enocom · 2018-06-05T15:22:52Z

samples/guestbook/inject_gcp.go

+	runtimeConfigVar  = "motd"
+)
+
+func setupGCP(ctx context.Context) (*app, func(), error) {


Same as above. Documentation here and for all the functions below.

enocom · 2018-06-05T15:23:50Z

samples/guestbook/inject_local.go

+	motdFile      = "motd.txt"
+)
+
+func setupLocal(ctx context.Context) (*app, func(), error) {


Some documentation here as well. "setupLocal creates bindings which allow for local development independent of any IaaS resources" or similar.

enocom · 2018-06-05T15:25:28Z

samples/guestbook/main.go

+	flag.Parse()
+
+	ctx := context.Background()
+	var app *app


Stutter in the variable name. The type already tells us it's an app. Since app is used only in this function, a comes across as idiomatic to me.

Alternatively, change the type name to application and use app as the variable name.

Renamed to application.

enocom · 2018-06-05T15:29:52Z

samples/guestbook/main.go

+
+// app is the main server struct for Guestbook.
+type app struct {
+	backends


A pluralized noun for a single type seems strange.

See below, I've removed the backends type.

enocom · 2018-06-05T15:32:33Z

samples/guestbook/main.go

+	motd string // message of the day
+}
+
+// backends is a set of platform-independent services that the Guestbook uses.


I see the word "set" but see nothing in the fields which indicates there is more than one Server. Do you mean that srv, db, and bucket each represent an IaaS-specific resource? Why have the separate type instead of just putting them directly on app? A little more explanation in the documentation on the "why" would be helpful here.

I've removed backends struct because it is indeed confusing. It was mostly an attempt to make it "one line to add new dependencies". Upon reflection, it is definitely contorting the application structure to show off a neat trick with Wire, but it does not help with clarity.

zombiezen · 2018-06-05T22:55:57Z

Thank you all for the feedback! I'm going to move the platform setup out of the README and into a Terraform configuration (kudos to @enocom for pointing me to it!), which should alleviate a lot of the "big wall o' CLI" concerns, as well as making the setup much more maintainable. I'll ping back here when I've done this and addressed the other comments.

…ation

zombiezen

PTAL. I believe I've made most of the requested changes, although I haven't had a chance to re-test everything with the new setup.

zombiezen · 2018-06-05T22:52:34Z

samples/guestbook/README.md

+select.
+
+```shell
+$ PROJECT_ID=...


Discussed over IM. I didn't know that Terraform was exactly the thing I needed, so I've rewritten this with Terraform. Thanks!

zombiezen · 2018-06-05T22:53:10Z

samples/guestbook/README.md

+$ gcloud iam service-accounts delete $GCP_IAM_ACCOUNT
+```
+
+## Running on Amazon Web Services (AWS)


Obsoleted by Terraform rewrite.

zombiezen · 2018-06-07T04:06:54Z

samples/guestbook/README.md

@@ -0,0 +1,266 @@
+# Guestbook Sample
+


zombiezen · 2018-06-07T04:09:15Z

samples/guestbook/README.md

+# Guestbook Sample
+
+## Building
+


zombiezen · 2018-06-07T04:10:28Z

samples/guestbook/README.md

+```
+
+## Running Locally
+


zombiezen · 2018-06-07T17:00:32Z

samples/guestbook/main.go

+	flag.Parse()
+
+	ctx := context.Background()
+	var app *app


Renamed to application.

zombiezen · 2018-06-07T17:06:51Z

samples/guestbook/main.go

+
+// app is the main server struct for Guestbook.
+type app struct {
+	backends


See below, I've removed the backends type.

zombiezen · 2018-06-07T17:08:41Z

samples/guestbook/main.go

+	motd string // message of the day
+}
+
+// backends is a set of platform-independent services that the Guestbook uses.


I've removed backends struct because it is indeed confusing. It was mostly an attempt to make it "one line to add new dependencies". Upon reflection, it is definitely contorting the application structure to show off a neat trick with Wire, but it does not help with clarity.

zombiezen · 2018-06-07T17:09:18Z

samples/guestbook/main.go

+	}
+}
+
+// index serves the server's landing page.


Not sure how this happened. 😄 Thanks!

zombiezen · 2018-06-07T17:36:24Z

samples/guestbook/main.go

+		return
+	}
+	defer q.Close()
+	for q.Next() {


I read your suggestion slightly incorrectly and filed golang/go#25787. There's a bit too much mutable state for me in your suggestion, so I'm going to keep it as is.

enocom · 2018-06-08T01:54:02Z

Very nice. Looks much cleaner and more user-friendly. I'll do a test run through the tutorial to verify all the instructions work and so on.

enocom

There's a syntax error in the script starting the local DB and as permissions stand in a new project, the terraform apply step fails.

enocom · 2018-06-08T16:28:37Z

samples/guestbook/README.md

+Guestbook is a sample application that records visitors' messages, displays a
+cloud banner, and an administrative message. The main business logic is
+written in a cloud-agnostic manner using MySQL, the generic blob API, and the
+generic runtimevar API. Each of the platform-specific code is set up by Wire.


"All platform-specific code" reads better to me.

enocom · 2018-06-08T16:44:38Z

samples/guestbook/start-localdb.sh

@@ -0,0 +1,52 @@
+#!/bin/bash


The executable bits should be flipped on this file.

Here's the current state:

-rw-r--r-- 1 enocom eng 1568 Jun 8 10:16 start-localdb.sh

enocom · 2018-06-08T16:49:26Z

samples/guestbook/start-localdb.sh

+  --interactive \
+  --link "${container_name}:mysql" \
+  "$image" \
+  sh -c 'exec mysql -h"$MYSQL_PORT_3306_TCP_ADDR" -P"$MYSQL_PORT_3306_TCP_PORT" -uroot -ppassword guestbook' || { \


There is a syntax error on this line and the script currently does not run.

Make sure to run shellcheck against all bash scripts to make sure their in tip-top shape.

cf. https://github.com/koalaman/shellcheck

shellcheck is more magical than I expected and I am pleased. Thank you again for showing me the coolest tools. 👍

enocom · 2018-06-08T16:50:34Z

samples/guestbook/README.md

+./start-localdb.sh
+./guestbook -env=local
+```
+


After it's all turned on, what do I do? What address do I visit in my browser?

enocom · 2018-06-08T16:51:45Z

samples/guestbook/README.md

+can run the server.
+
+```shell
+./start-localdb.sh


To avoid the pain of users forgetting to stop their container, you could make this a foregrounded process and tell the user to open a new session, tab, window, etc.

enocom · 2018-06-08T16:58:03Z

samples/guestbook/README.md

+gcloud auth application-default login
+cd gcp
+terraform init
+terraform apply


Terraform prompts me for a project, a region, and a zone. Might be helpful to add a one-liner about what a user should enter to ease any friction.

enocom · 2018-06-08T16:59:47Z

samples/guestbook/README.md

+gcloud auth application-default login
+cd gcp
+terraform init
+terraform apply


Here's what I get when I follow this verbatim using my personal account on GCP:

Error: Error applying plan: 6 error(s) occurred: * google_project_service.runtimeconfig: 1 error(s) occurred: * google_project_service.runtimeconfig: Error enabling service: failed to issue request: googleapi: Error 403: The caller does not have permission, forbidden * google_project_service.storage: 1 error(s) occurred: * google_project_service.storage: Error enabling service: failed to issue request: googleapi: Error 403: The caller does not have permission, forbidden * google_service_account.db_access: 1 error(s) occurred: * google_service_account.db_access: Error creating service account: googleapi: Error 403: Permission iam.serviceAccounts.create is required to perform this operation on project projects/go-cloud-test., forbidden * google_service_account.server: 1 error(s) occurred: * google_service_account.server: Error creating service account: googleapi: Error 403: Permission iam.serviceAccounts.create is required to perform this operation on project projects/go-cloud-test., forbidden * google_project_service.sql: 1 error(s) occurred: * google_project_service.sql: Error enabling service: failed to issue request: googleapi: Error 403: The caller does not have permission, forbidden * google_project_service.container: 1 error(s) occurred: * google_project_service.container: Error enabling service: failed to issue request: googleapi: Error 403: The caller does not have permission, forbidden

I believe this is hashicorp/terraform-provider-google#1579. I was getting it too and was about to start bothering folks.

enocom · 2018-06-08T17:03:53Z

The use of terraform is a big win, though. It's going to be a big improvement over raw commands.

zombiezen

PTAL

zombiezen · 2018-06-13T16:39:51Z

samples/guestbook/README.md

+gcloud auth application-default login
+cd gcp
+terraform init
+terraform apply


I believe this is hashicorp/terraform-provider-google#1579. I was getting it too and was about to start bothering folks.

zombiezen · 2018-06-13T16:41:38Z

samples/guestbook/README.md

+Guestbook is a sample application that records visitors' messages, displays a
+cloud banner, and an administrative message. The main business logic is
+written in a cloud-agnostic manner using MySQL, the generic blob API, and the
+generic runtimevar API. Each of the platform-specific code is set up by Wire.


zombiezen · 2018-06-13T16:42:36Z

samples/guestbook/README.md

+./start-localdb.sh
+./guestbook -env=local
+```
+


zombiezen · 2018-06-13T16:44:08Z

samples/guestbook/README.md

+gcloud auth application-default login
+cd gcp
+terraform init
+terraform apply


zombiezen · 2018-06-13T16:45:55Z

samples/guestbook/start-localdb.sh

@@ -0,0 +1,52 @@
+#!/bin/bash


zombiezen · 2018-06-13T17:09:10Z

samples/guestbook/start-localdb.sh

+  --interactive \
+  --link "${container_name}:mysql" \
+  "$image" \
+  sh -c 'exec mysql -h"$MYSQL_PORT_3306_TCP_ADDR" -P"$MYSQL_PORT_3306_TCP_PORT" -uroot -ppassword guestbook' || { \


shellcheck is more magical than I expected and I am pleased. Thank you again for showing me the coolest tools. 👍

enocom

There are a couple more issues with the service account permissions on GCP. I didn't test AWS. Otherwise, once the permissions are fixed, I think this is ready to merge.

enocom · 2018-06-13T21:48:00Z

samples/guestbook/README.md

+In another terminal, you can run:
+
+```shell
+./guestbook -env=local


On a fresh clone after following all instructions up to this point, I get this:

$ ./guestbook -env=local 2018/06/13 15:45:16 open file bucket: stat : no such file or directory

enocom · 2018-06-13T21:48:26Z

samples/guestbook/README.md

+
+Your server should be running on http://localhost:8080/.
+
+You can stop the MySQL database server with Ctrl-\. MySQL ignores Ctrl-C


This is a very nice touch. I would have mashed ctrl-c endlessly otherwise.

enocom · 2018-06-13T21:52:54Z

samples/guestbook/README.md

+
+# Terraform will prompt you for your GCP project ID, desired region,
+# and desired zone.
+terraform apply


Granted, I know how to fix this myself by enabling the API (i.e., gcloud services enable container.googleapis.com) and adjusting IAM permissions, but it would be nice if README told me to do this or if the terraform config was updated to ensure permissions worked out of the box.

After running terraform apply on a vanilla setup against a fresh account, I get this:

3 error(s) occurred: * google_project_service.storage: 1 error(s) occurred: * google_project_service.storage: Error enabling service: failed to issue request: googleapi: Error 403: The caller does not have permission, forbidden * google_project_service.sql: 1 error(s) occurred: * google_project_service.sql: Error enabling service: failed to issue request: googleapi: Error 403: The caller does not have permission, forbidden * google_project_service.container: 1 error(s) occurred: * google_project_service.container: Error enabling service: Error waiting for apis ["container.googleapis.com"] to be enabled for enocom-dev: googleapi: Error 403: The caller does not have permission, forbidden

Ironically, I believe these messages come up because the services are already enabled. :\ Tracking bug is hashicorp/terraform-provider-google#1579

enocom

LGTM. Thanks for your patience.

jeanbza

Looks very nice!! Nice one Ross 👍

zombiezen · 2018-06-13T23:14:38Z

@cflewis, any idea why I'm getting this weird (merge-blocking) failure? https://travis-ci.com/google/go-cloud/builds/76231027#L632

cflewis · 2018-06-14T15:56:35Z

It's almost certainly #58 .

I'll send a PR to just skip that test.

cflewis · 2018-06-14T16:02:37Z

Hmm, no, it's not #58. I think Travis flaked. There's no reason it shouldn't have any space on the device, the tests write tiny files. I kicked off the build again and we'll see what happens.

cflewis · 2018-06-14T16:05:24Z

OK, the build is happy now. Except that vgo is still broken. I see that there's a patch in Gerrit; is there a lag time from Gerrit to GitHub or is it mirrored immediately? If not, we should just update the Travis script to stop running vendor in the meantime.

zombiezen · 2018-06-14T17:03:17Z

The vgo fix (https://golang.org/cl/118757) unmasks a different failure that @rsc is actively debugging (the spew/testdata failure).

zombiezen · 2018-06-14T19:34:16Z

vgo fix is coming in golang.org/cl/118956.

@cflewis

@cflewis: "It doesn't even detect dead methods." Unblocks #68

Unblocks #68

samples/guestbook: add sample application

7bedc38

zombiezen requested review from enocom, jeanbza, jba and tbpg June 4, 2018 20:06

zombiezen added 2 commits June 4, 2018 13:08

samples/guestbook: fix binary name in Dockerfile

137ad82

samples/guestbook: handle ignored error

ad7433b

Thanks, linter!

jeanbza suggested changes Jun 5, 2018

View reviewed changes

jba reviewed Jun 5, 2018

View reviewed changes

enocom reviewed Jun 5, 2018

View reviewed changes

enocom suggested changes Jun 5, 2018

View reviewed changes

zombiezen added 4 commits June 6, 2018 21:02

Add Terraform configurations for GCP and AWS

fb0d797

fill out more of README

0146eb7

Document Terraform variables.

6c23658

terraform fmt

9f6c32c

zombiezen mentioned this pull request Jun 7, 2018

tests: AWS integration test harness #17

Closed

Use command-line flags instead of compile-time constants for configur…

f329ef6

…ation

zombiezen commented Jun 8, 2018

View reviewed changes

enocom suggested changes Jun 8, 2018

View reviewed changes

zombiezen added 2 commits June 13, 2018 09:45

Make start-localdb.sh executable

c1cd866

Run scripts through shellcheck

c6da8e9

Explain required user input in README

827fc83

zombiezen commented Jun 13, 2018

View reviewed changes

zombiezen added this to the Sprint 13 milestone Jun 13, 2018

enocom suggested changes Jun 13, 2018

View reviewed changes

zombiezen added 2 commits June 13, 2018 15:27

Add directions around creating the local bucket

2bb9f57

document local MOTD

644086c

enocom approved these changes Jun 13, 2018

View reviewed changes

Merge branch 'master' into guestbook

dc9029d

jeanbza approved these changes Jun 13, 2018

View reviewed changes

zombiezen mentioned this pull request Jun 14, 2018

testing: allow dead code in PRs #93

Merged

zombiezen added a commit that referenced this pull request Jun 14, 2018

testing: allow dead code in PRs (#93)

6ed7c36

@cflewis: "It doesn't even detect dead methods." Unblocks #68

Merge branch 'master' into guestbook

feec1e4

zombiezen mentioned this pull request Jun 14, 2018

testing: allow _unused_ code in PRs #94

Merged

cflewis pushed a commit that referenced this pull request Jun 14, 2018

testing: allow _unused_ code in PRs (#94)

5560d81

Unblocks #68

Merge branch 'master' into guestbook

c4fda21

zombiezen merged commit 2f80740 into google:master Jun 14, 2018

zombiezen deleted the guestbook branch June 14, 2018 21:45


		Your server should be running on http://localhost:8080/.

		You can stop the MySQL database server with Ctrl-\. MySQL ignores Ctrl-C

samples/guestbook: add sample application #68

samples/guestbook: add sample application #68

Conversation

zombiezen commented Jun 4, 2018

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

enocom left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

zombiezen commented Jun 5, 2018

zombiezen left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

enocom commented Jun 8, 2018

enocom left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment